Skip to content

feat(naive_bayes): dataframe-agnostic MultinomialNB via narwhals - #1974

Open
JokeGbenro wants to merge 6 commits into
online-ml:mainfrom
JokeGbenro:feat/multinomial-nb-narwhals
Open

feat(naive_bayes): dataframe-agnostic MultinomialNB via narwhals#1974
JokeGbenro wants to merge 6 commits into
online-ml:mainfrom
JokeGbenro:feat/multinomial-nb-narwhals

Conversation

@JokeGbenro

Copy link
Copy Markdown

Summary

Fixes #1919

  • Updates MultinomialNB.learn_many and joint_log_likelihood_many to support Narwhals-compatible dataframe inputs.
  • Adds a backend-agnostic BaseNB.predict_many implementation.
  • Adds tests covering MultinomialNB mini-batch prediction behavior across supported dataframe backends.

Testing

python -m ruff check river\naive_bayes\base.py river\naive_bayes\multinomial.py river\naive_bayes\test_naive_bayes.py
python -m pytest river/naive_bayes/test_naive_bayes.py -q

Co-authored-by: Codex <codex@openai.com>
@JokeGbenro
JokeGbenro force-pushed the feat/multinomial-nb-narwhals branch from 45dea61 to 6e0afad Compare August 1, 2026 16:59
@codspeed-hq

codspeed-hq Bot commented Aug 1, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 98 untouched benchmarks
⏩ 16 skipped benchmarks1


Comparing JokeGbenro:feat/multinomial-nb-narwhals (a52e293) with main (fe248b4)

Open in CodSpeed

Footnotes

  1. 16 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@JokeGbenro

Copy link
Copy Markdown
Author

Hi @MaxHalford @smastelini

I pushed two follow-up fixes for the failing checks.

The pandas doctest was failing because BaseNB.predict_many forced predicted labels into an object array, which changed GaussianNB.predict_many output from dtype: int64 to dtype: object. I removed the forced object dtype so numeric labels keep their native dtype.

The prek failure was from mypy in MultinomialNB.learn_many, where y was reused for both the input series and the one-hot sparse target matrix. I split that into y_one_hot and added the return annotation for _one_hot_targets.

Verified locally:

  • python -m pytest river/naive_bayes -q
  • python -m ruff check river\naive_bayes\multinomial.py
  • targeted mypy check on river\naive_bayes\multinomial.py

@MaxHalford

Copy link
Copy Markdown
Member

Could you fix the conflicts?

@JokeGbenro

Copy link
Copy Markdown
Author

Hi @MaxHalford, I’ve resolved the merge conflicts.

The Naive Bayes tests have been kept in their new tests/naive_bayes location, and the fixture import was updated to use tests.frames. I also verified that there are no remaining conflict markers and that the changed Python files compile successfully.

@MaxHalford

Copy link
Copy Markdown
Member

Ok good! Can you add a release note to unreleased.md?

@JokeGbenro

Copy link
Copy Markdown
Author

Hi @MaxHalford, added the release note in unreleased.md under naive_bayes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate all mini-batch (_many) methods to narwhals for dataframe-agnostic support

2 participants